A database outage should not be reported as 401 Unauthorized - #59

The problem

Authentication reads the database. So does every route behind it. When the
database is unreachable, authenticateApiKeyMiddleware catches the resulting
exception and answers 401:

} catch (err: any) {
  if (!res.headersSent) {
    return res.status(401).json({
      error: err.message || 'Invalid API Key',
    });
  }
}

which produces things like:

401 {"error":"connect ECONNREFUSED 127.0.0.1:27017"}
401 {"error":"Operation `users.findOne()` buffering timed out after 10000ms"}

The status says the caller sent a bad key. The body says Mongoose could not
reach the database. Only the body is true, and most clients only look at the
status.

Meanwhile /api/v1/healthcheck returns 200 unconditionally — it proves the
HTTP listener is up and nothing else.

Why it matters

I lost most of a day to this. A SPACE instance had been up for fifteen hours,
its healthcheck passing the whole time, with its mongodb container stopped.
Every authenticated request came back 401, so the first assumption was a wrong
API key — then a wrong admin password, then a mis-copied organization key. The
healthcheck confirmed, repeatedly, that the service was fine.

Two signals agreeing on the wrong answer is what made it expensive. Either one
alone would have been survivable.

It also defeats an orchestrator: a container that reports healthy while unable
to serve a single request is one Kubernetes or Compose will keep sending
traffic to, and will never restart.

The change

Authentication. authenticateUserApiKey and authenticateOrgApiKey throw a
typed InvalidApiKeyError for the two cases that really are bad credentials
(no such user, no such organization). The catch answers 401 only for those.
Anything else is a 503 with Retry-After, because the credential was never
judged — the request did not get that far — and the caller should try again
rather than go looking at their key. The underlying error is logged and
included as details.

Healthcheck. 503 when Mongoose is not connected, and when it is, a ping
before answering 200: readyState is what the driver believes, a ping is what
the database says, and the two disagree when a connection has gone stale. The
response now carries database: "connected" | "disconnected" | "unreachable".

Verification

Stopping the database under a running server, same request both ways:

main:
  healthcheck HTTP 200 -> {"message":"Service is up and running!"}
  authed call HTTP 401 -> {"error":"connect ECONNREFUSED ::1:27117, connect ECONNREFUSED 127.0.0.1:27117"}

this PR:
  healthcheck HTTP 503 -> {"message":"Service is up but cannot reach its database.","database":"disconnected"}
  authed call HTTP 503 -> {"error":"Space cannot verify credentials right now.","details":"connect ECONNREFUSED …"}

With the database up, both branches behave identically: healthcheck 200, and an
unknown key still 401 — see the correction below, which is where that claim
first went wrong.

pnpm run build (tsc) passes.

A note on scope

I have deliberately not touched the other catch blocks that map errors to
statuses, nor added a readiness probe distinct from liveness — either could be
worth doing, but this PR is about the one pair of signals that actively
misleads. Happy to extend it if you would like the wider change.

Correction and follow-up commit

The first version of this PR broke a 401. I ran the repository's full suite
per file afterwards and authMiddleware.test.ts failed:

× Should return 401 with non-existent user API key
AssertionError: expected 503 to be 401

My original claim above that "an unknown key still 401" was checked by hand
against an organization key and did not hold for the user path. The reason:

// UserService.findByApiKey
const user = await this.userRepository.findByApiKey(apiKey);
if (!user) {
  throw new Error('INVALID DATA: Invalid API Key');   // reports absence by throwing
}

The service never returns nothing for an unknown key — it throws a plain
Error. So the if (!user) throw new InvalidApiKeyError(...) guard I added was
unreachable, and the real rejection fell through to the new 503 branch.
Narrowing the catch to a dedicated error type is not by itself sufficient when
the failure being narrowed away is signalled the same way.

The follow-up commit wraps both credential lookups in one place that tells the
codebase's own convention apart from everything else — INVALID DATA: is the
prefix this repository already uses for a caller's mistake, so it stays a 401,
and anything else is the database being unable to answer:

async function rejectionOrOutage<T>(lookup: () => Promise<T>, absent: string): Promise<T> {
  let found: T;
  try {
    found = await lookup();
  } catch (err: any) {
    if (typeof err?.message === 'string' && err.message.startsWith('INVALID DATA:')) {
      throw new InvalidApiKeyError(err.message);
    }
    throw err;
  }
  if (!found) throw new InvalidApiKeyError(absent);
  return found;
}

Verification of the follow-up

api/src/test/middlewares/authOutage.test.ts is new and pins both
directions, since the whole change is about the boundary between them:

✓ src/test/middlewares/authOutage.test.ts (4 tests)
  ✓ answers 503, not 401, when the lookup fails
  ✓ asks the caller to try again
  ✓ still answers 401 to a key that was read and rejected
  ✓ still answers 401 to a key of no recognisable kind

Each direction was confirmed load-bearing by reverting the source:

The suite this PR broke is green again: authMiddleware.test.ts, 68 passed.
npx tsc --noEmit is clean.